Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RNET-1145, RNET-1143: Fix reading schema from native #3601

Merged
merged 4 commits into from
May 21, 2024
Merged

Conversation

nirinchev
Copy link
Member

Description

This fixes how we're reading Realm schema from native. It seems that for a few string properties, the default value in Core is empty string, whereas in .NET those are nullable, so our checks for object_type == null were failing and we were trying to lookup metadata for primitive properties.

Additionally, this adds Migration.FindInNewRealm since I saw we're missing that, which made the test easier to write.

Fixes #3600
Fixes #3597

TODO

  • Changelog entry
  • Tests

@cla-bot cla-bot bot added the cla: yes label May 7, 2024
@nirinchev nirinchev requested a review from papafe May 7, 2024 22:45
Copy link

coveralls-official bot commented May 9, 2024

Pull Request Test Coverage Report for Build 9181337499

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 22 of 22 (100.0%) changed or added relevant lines in 5 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 80.16%

Files with Coverage Reduction New Missed Lines %
Realm/Realm/Native/PrimitiveValue.cs 1 79.66%
Realm/Realm/Configurations/RealmConfigurationBase.cs 1 95.35%
Totals Coverage Status
Change from base Build 8937828998: 0.02%
Covered Lines: 6739
Relevant Lines: 8258

💛 - Coveralls

Copy link
Contributor

@papafe papafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just left a small comment and maybe we should fix those warnings about the non-nullable property

{
ThrowIfDisposed();
Argument.Ensure(obj.IsManaged, "Only managed objects can be used in FindExisting", nameof(obj));
var metadata = Metadata[typeof(T).GetMappedOrOriginalName()];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small optimization, but maybe we can move this inside the if?

@nirinchev nirinchev merged commit 02bc7dd into main May 21, 2024
19 checks passed
@nirinchev nirinchev deleted the ni/dotnet-3597 branch May 21, 2024 20:55
nirinchev added a commit that referenced this pull request May 22, 2024
* main:
  RNET-1134: Add obfuscation attribute on generated RealmSchema (#3594)
  Revert "Hide collections in mixed from public API" (#3606)
  Update to Core 14.7.0 (#3605)
  RNET-1145, RNET-1143: Fix reading schema from native (#3601)

# Conflicts:
#	.github/templates/test-weaver.yml
#	.github/workflows/test-weaver.yml
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
2 participants